fix(tanstack-query): propagate TPageParam generic through useInfiniteFindMany#2561
Merged
fix(tanstack-query): propagate TPageParam generic through useInfiniteFindMany#2561
Conversation
…FindMany Adds TPageParam generic to ModelInfiniteQueryOptions, ModelSuspenseInfiniteQueryOptions, and the useInfiniteFindMany/useSuspenseInfiniteFindMany interface signatures across React, Vue, and Svelte adapters, so that lastPageParam and pageParams are properly typed instead of falling back to unknown. Fixes #2426. Co-authored with @motopods (motopods#1). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Claude finished @ymc9's task —— View job PR Review: TPageParam Generic Propagation ✅Reviewed PR #2561 that fixes TPageParam generic inference in TanStack Query integration. Review Checklist:
Analysis & Feedback✅ Code Quality & Best Practices
✅ Bug Fixes & Type Safety
✅ Implementation DetailsReact (react.ts:122-136):
Vue (vue.ts:121-129):
Svelte (svelte/index.svelte.ts:128-136):
✅ Test Coverage
Key test validation from react-typing.test-d.ts:41-48: const infiniteResult = client.user.useInfiniteFindMany(
{},
{
getNextPageParam: (_lastPage, _allPages, lastPageParam: { cursor: string }) => lastPageParam,
},
);
check(infiniteResult.data?.pageParams[0]?.cursor); // Now properly typed!✅ Performance & Security
✅ Areas of Excellence
SummaryThis is a well-executed, focused fix that solves the TPageParam inference issue across all supported frameworks. The implementation is clean, consistent, and maintains backward compatibility while providing the improved type safety users need for infinite queries. Recommendation: ✅ Approve - Ready for merge. |
Contributor
📝 WalkthroughWalkthroughAdded a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/clients/tanstack-query/src/vue.ts (1)
516-552:⚠️ Potential issue | 🔴 CriticalUse
pageParamfor follow-up pages in the Vue adapter.Line 547 still builds the request from
argsValue, sofetchNextPagekeeps refetching the first page even aftergetNextPageParamcomputes a new page param. This PR fixes the types, but pagination is still broken at runtime unless the query function usespageParam ?? argsValue.🛠️ Suggested fix
const finalOptions: any = computed(() => { const argsValue = toValue(args); return { queryKey: queryKey.value, - queryFn: ({ signal }: any) => { - const reqUrl = makeUrl(endpoint, model, operation, argsValue); + queryFn: ({ pageParam, signal }: any) => { + const reqUrl = makeUrl(endpoint, model, operation, pageParam ?? argsValue); return fetcher<TQueryFnData>(reqUrl, { signal }, fetch); }, initialPageParam: toValue(argsValue) as TPageParam, ...toValue(options), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/clients/tanstack-query/src/vue.ts` around lines 516 - 552, The infinite query function useInternalInfiniteQuery builds requests from argsValue so fetchNextPage always refetches page 1; update the computed finalOptions.queryFn in useInternalInfiniteQuery to accept the infinite query context (including pageParam) and use pageParam ?? argsValue when calling makeUrl/fetch so follow-up pages use the computed pageParam; keep initialPageParam as toValue(args) but ensure the queryFn signature destructures pageParam (and signal) and uses that value to build the reqUrl.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/clients/tanstack-query/src/react.ts`:
- Around line 266-274: The generic TPageParam for useInfiniteFindMany and
useSuspenseInfiniteFindMany must be constrained to the seeded findMany args so
callers cannot supply an unrelated page-param type; update the signatures for
useInfiniteFindMany and useSuspenseInfiniteFindMany so TPageParam is derived
from the provided args (e.g., by making TPageParam extend the cursor/seed type
extracted from the SelectSubset<T, FindManyArgs<...>> or by deriving it from T
via a helper type) and remove the unsafe "as TPageParam" casts used when passing
initialPageParam/lastPageParam; ensure the resulting constraint ties TPageParam
to the shape produced by the findMany args (references: useInfiniteFindMany,
useSuspenseInfiniteFindMany, TPageParam, FindManyArgs, initialPageParam,
lastPageParam).
In `@packages/clients/tanstack-query/src/svelte/index.svelte.ts`:
- Around line 249-256: The Svelte infinite-query path allows args to be omitted
but the implementation calls args() unconditionally; update
useInternalInfiniteQuery and any call sites used by useInfiniteFindMany (e.g.,
where query key or fetch function is built at the spots referenced around
useInternalInfiniteQuery) to accept an optional Accessor and invoke it safely as
args?.() (matching useInternalQuery's pattern), ensuring any downstream logic
that expects an object handles undefined (e.g., defaulting to {} or skipping
select/build when args is undefined) so a plain useInfiniteFindMany() doesn't
throw before the query is created.
---
Outside diff comments:
In `@packages/clients/tanstack-query/src/vue.ts`:
- Around line 516-552: The infinite query function useInternalInfiniteQuery
builds requests from argsValue so fetchNextPage always refetches page 1; update
the computed finalOptions.queryFn in useInternalInfiniteQuery to accept the
infinite query context (including pageParam) and use pageParam ?? argsValue when
calling makeUrl/fetch so follow-up pages use the computed pageParam; keep
initialPageParam as toValue(args) but ensure the queryFn signature destructures
pageParam (and signal) and uses that value to build the reqUrl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47dfdeae-96a0-4f94-b9cd-e7a07a44745f
📒 Files selected for processing (6)
packages/clients/tanstack-query/src/react.tspackages/clients/tanstack-query/src/svelte/index.svelte.tspackages/clients/tanstack-query/src/vue.tspackages/clients/tanstack-query/test/react-typing.test-d.tspackages/clients/tanstack-query/test/svelte-typing-test.tspackages/clients/tanstack-query/test/vue-typing-test.ts
Contributor
|
Claude finished @ymc9's task —— View job PR Review: TPageParam Generic Propagation ✅Reviewed PR #2561 that fixes TPageParam generic inference in TanStack Query integration. Review Checklist:
Analysis & Feedback✅ Code Quality & Best Practices
✅ Bug Fixes & Type Safety
✅ Implementation DetailsReact (react.ts:122-136):
Vue (vue.ts:121-129):
Svelte (svelte/index.svelte.ts:128-136):
✅ Test Coverage
Key test validation from react-typing.test-d.ts:41-48: const infiniteResult = client.user.useInfiniteFindMany(
{},
{
getNextPageParam: (_lastPage, _allPages, lastPageParam: { cursor: string }) => lastPageParam,
},
);
check(infiniteResult.data?.pageParams[0]?.cursor); // Now properly typed!✅ Performance & Security
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/clients/tanstack-query/src/svelte/index.svelte.ts`:
- Around line 249-256: The generic TPageParam on useInfiniteFindMany is
unconstrained and allows incompatible types to be passed to makeUrl and
initialPageParam; update the useInfiniteFindMany signature so TPageParam extends
the FindManyArgs shape (i.e., TPageParam extends FindManyArgs<Schema, Model,
Options, {}, ExtResult> | undefined) and set its default to T | undefined so
inference matches args; then remove the unsafe cast at initialPageParam and
ensure pageParam is passed directly to makeUrl with the new constrained type
(references: useInfiniteFindMany, makeUrl, and the initialPageParam usage).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60d67cc8-fa81-472e-9776-a43fa7778b91
📒 Files selected for processing (1)
packages/clients/tanstack-query/src/svelte/index.svelte.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds TPageParam generic to ModelInfiniteQueryOptions, ModelSuspenseInfiniteQueryOptions, and the useInfiniteFindMany/useSuspenseInfiniteFindMany interface signatures across React, Vue, and Svelte adapters, so that lastPageParam and pageParams are properly typed instead of falling back to unknown.
Fixes #2426. Co-authored with @motopods (motopods#1).
Summary by CodeRabbit